Add support for empty input lists#68
Conversation
ytsarev
left a comment
ytsarev
left a comment
There was a problem hiding this comment.
@tvandinther thanks a lot for the PR. It is useful! Could you please add unit tests that cover the logic?
The current test suite mocks the whole query function with an implementation which performs the same check as what I have modified in the real implementation. https://github.com/upbound/function-msgraph/blob/main/fn_test.go#L374 Unsure how to proceed exactly since these changes sit below the mocked interface. I considered exiting early but that is technically a different implementation as the target should continue to have (empty) results written and status should continue as usual, rather than the query being skipped altogether. |
|
For now I've updated the mocks to have the same changes as in the real implementation to show that these changes work as intended, even though the code is not in the test path. |
tvandinther
force-pushed
the
empty-input-lists
branch
from
f5a6033 to
6645d86
Compare
With this change, if no groups are queried, an empty list is returned instead of null. Signed-off-by: Tom van Dinther <39470469+tvandinther@users.noreply.github.com>
Also updates the mock to contain the same logic as added in the real implementation. Signed-off-by: Tom van Dinther <39470469+tvandinther@users.noreply.github.com>
Signed-off-by: Tom van Dinther <39470469+tvandinther@users.noreply.github.com>
tvandinther
force-pushed
the
empty-input-lists
branch
from
380ff9e to
c0db281
Compare
ytsarev
left a comment
ytsarev
left a comment
There was a problem hiding this comment.
Can you please add example to README and document new configuration field in https://github.com/upbound/function-msgraph?tab=readme-ov-file#input-configuration-options ? Otherwise, I believe it is good to merge 👍
Signed-off-by: Tom van Dinther <39470469+tvandinther@users.noreply.github.com>
|
Thanks @ytsarev, README update has been pushed. |
ytsarev
left a comment
ytsarev
left a comment
There was a problem hiding this comment.
Awesome! @tvandinther thank you so much for the contribution!
2 participants
Description of your changes
With the changes in this MR, the function now completes successfully with empty input lists or refs which resolve to empty lists.
Added a new optional boolean field on the function input named
failOnEmpty. This field opts into the previous behaviour of this function in case it is desired.Fixes #67
I have: